Skip to content

refactor: Move socket functions to buddySocket file #744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

refactor: Move socket functions to buddySocket file #744

wants to merge 3 commits into from

Conversation

yuvrajinbhakti
Copy link

@yuvrajinbhakti yuvrajinbhakti commented Feb 11, 2025 •
edited
Loading

Fixes Issue

My PR closes #661

👨‍💻 Changes proposed(What did you do ?)

✔️ Check List (Check all the applicable boxes)

  • Moved all socket-related functionality from BuddyMatcher component into a separate buddySocket.js file
  • Created a new custom hook useBuddySocket to encapsulate socket logic
  • Fixed JSX syntax issues by ensuring proper file extensions
  • Improved code organization and maintainability
  • Removed JSX content from .js files to prevent syntax errors

✔️ Check List

  • [✔️] My code follows the code style of this project.
  • [✔️] This PR does not contain plagiarized content.
  • [✔️] The title and description of the PR is clear and explains the approach.

Note to reviewers

  • The main functional change is moving socket logic to a separate file
  • Special attention needed for the socket event cleanup in useEffect
  • Verified that all socket events are properly handled
  • Ensured no JSX syntax in .js files
  • No change in core functionality, this is purely a refactoring PR

@vercel Vercel
Copy link

vercel bot commented Feb 11, 2025

@yuvrajinbhakti is attempting to deploy a commit to the dunsin's projects Team on Vercel.

A member of the Team first needs to authorize it.

github-actions[bot]
github-actions bot reviewed Feb 11, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Compliance Checks Passed!

Dun-sin
Dun-sin requested changes Feb 12, 2025
View reviewed changes
Copy link
Owner

@Dun-sin Dun-sin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, can you share a recording of the working software? With you chatting

client/src/App.jsx Outdated
import Profile from './pages/Profile';
import ProtectedRoutes from 'components/ProtectedRoutes';
import ReactGA from 'react-ga4';
// import ReactGA from 'react-ga4';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please uncomment this

Dun-sin
Dun-sin reviewed Mar 4, 2025
View reviewed changes
Dun-sin
Dun-sin reviewed Mar 4, 2025
View reviewed changes
@Dun-sin
Copy link
Owner

Dun-sin commented Mar 4, 2025

@yuvrajinbhakti are you still interested in this? i asked for a recording of the working project on your local computer with your change, please get it done by friday else i would have to close your PR. Thanks

@yuvrajinbhakti
Copy link
Author

@Dun-sin When attempting to install dependencies for the backend (in the server folder), the required dependency is not available on npm.
image

@t-trafer
Copy link
Contributor

t-trafer commented Mar 4, 2025

lgtm

@Dun-sin
Copy link
Owner

Dun-sin commented Mar 5, 2025

@Dun-sin When attempting to install dependencies for the backend (in the server folder), the required dependency is not available on npm. image

@Dun-sin When attempting to install dependencies for the backend (in the server folder), the required dependency is not available on npm. image

thanks for letting me know

@Dun-sin
Copy link
Owner

Dun-sin commented Mar 5, 2025

@Dun-sin When attempting to install dependencies for the backend (in the server folder), the required dependency is not available on npm. image

@Dun-sin When attempting to install dependencies for the backend (in the server folder), the required dependency is not available on npm. image

thanks for letting me know

@yuvrajinbhakti here's a walkaround for you, while we get that fix, remove that from your own package.json and then run npm install, if you don't set up a profile your server shouldn't crash

@yuvrajinbhakti
Copy link
Author

Sure, Will explore this workaround.

@Dun-sin
Copy link
Owner

Dun-sin commented Mar 17, 2025

Hi @yuvrajinbhakti this has been on for too long, will close it. If you think you can work on something else in the time given, feel free to comment on that new issue

@Dun-sin Dun-sin closed this Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@Dun-sin Dun-sin Dun-sin requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[OTHER] move all socket function into a different file
3 participants
@yuvrajinbhakti @Dun-sin @t-trafer